/ba-g test failures unrelated to build change

/ba-g test failures unrelated to build change

@steveisok what is the intended behavior when building the vmr? I assume we don't want to make ninja a required build dependency/assume it is available? Does it default to Ninja=false (during vmr build)?

Ninja could be optional for VMR, but it’s a lightweight package and improves build performance, so making it required may be worth considering.

It will only speed up a very small part of the vmr build, right?

Yes, it's about 10-15s saving and VMR cross build (e.g. on linux-riscv64) takes like 60-70m in GitHub Actions, so it's a tiny fraction. OTOH, ninja-build's download size is < 150 KB on Ubuntu and Fedora (and 66 KB on Alpine). 🤷‍♀️

about 10-15s saving and VMR cross build (e.g. on linux-riscv64) takes like 60-70m

If you're just building runtime, it's a nice gain.

For vmr builds, it's not significant.

Also, ninja isn't available everywhere.

$ podman run centos:stream9 dnf install ninja-build
CentOS Stream 9 - BaseOS                        4.6 MB/s | 8.9 MB     00:01    
CentOS Stream 9 - AppStream                     4.3 MB/s |  27 MB     00:06    
CentOS Stream 9 - Extras packages                26 kB/s |  20 kB     00:00    
No match for argument: ninja-build
Error: Unable to find a match: ninja-build

I think it makes sense to turn this off by default for the vmr build.

Also, ninja isn't available everywhere.

It's available in crb: dnf config-manager --enable crb && dnf install ninja-build

I don't think we should ninja a default build dependency for a 10-15s speedup on a 60-70min build.

Or alternatively, the build can automatically use ninja depending on whether it is installed.

automatically use ninja depending on whether it is installed.

That's how it's been working on mono for a while. It was discussed in the original PR #124041 (comment) and decision was made to make ninja on by default.

I'm assuming this was added to improve quality of life for the devs that works on runtime.

I don't think it was actively considered whether or not this was meant to be a default build dependency for vmr.

I find it to be only a very limited gain on the vmr build time to make it a default dependency.

cc @jkotas @jkoritzinsky

This was meant to be a default dependency for vmr too. We want to make sure that we are building the same by default everywhere.

It could be an option to consistently build the vmr with Ninja=false?

This was meant to be a default dependency for vmr too. We want to make sure that we are building the same by default everywhere.

I agree with Jan. I would prefer to default everywhere and opt out in the configurations that do not have it.